Skip to content
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

Remove support for relative path starting with / #148

Closed
wants to merge 7 commits into from
Closed

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Nov 25, 2015

@dnfclas
Copy link

dnfclas commented Nov 25, 2015

Hi @pakrym, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@pranavkm
Copy link
Contributor

You might also want to try this with Mvc. We pass paths starting with / all the time to the file system.

@pranavkm
Copy link
Contributor

Might also want to do something similar with EmbeddedFileProvider

@davidfowl
Copy link
Member

This also requires an announcement

@@ -25,7 +25,7 @@ public void ExistingFilesReturnTrue()
Assert.NotNull(info);
Assert.True(info.Exists);

info = provider.GetFileInfo("/File.txt");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a negative test making sure that passing /file does the appropriate thing on each OS.

@pakrym
Copy link
Contributor Author

pakrym commented Nov 30, 2015

Made same changes to embedded file system and added negative tests

@muratg
Copy link

muratg commented Dec 1, 2015

@pakrym What's the behavior for Windows absolute paths such as c:\foo\bar?

@pakrym
Copy link
Contributor Author

pakrym commented Dec 1, 2015

We returning NoopFileInfo

{
subpath = subpath.Substring(1);
}

// Non-hierarchal.
if (!subpath.Equals(string.Empty))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you combine this with the condition at Line 94?

@davidfowl
Copy link
Member

We decided to punt this and deal with the inconsistency.

@davidfowl davidfowl closed this Dec 7, 2015
@natemcmaster natemcmaster deleted the pakrym/relative branch March 14, 2017 19:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants